Skip to content

chore: Rust checks are required + merge queue#21941

Merged
comphead merged 1 commit intoapache:mainfrom
blaginin:db/mq-rust
May 6, 2026
Merged

chore: Rust checks are required + merge queue#21941
comphead merged 1 commit intoapache:mainfrom
blaginin:db/mq-rust

Conversation

@blaginin
Copy link
Copy Markdown
Member

@blaginin blaginin commented Apr 29, 2026

Closes #6880
Follow up on #17538 and #21239

#21239 seems to be working fine - time to make all the rust changes required to merge it.

This brings two cool things:

  1. The most important one - the "merge when ready" button now works 🚀
image

You can click this button any time, even right after opening a PR. It will be automatically merged when:

  • all required CI checks (which now include rust) are passing
  • you get an approval

I think this should help quite a bit. A lot of people get an approval, push a nit, and then sit waiting for CI to finish. No more waiting!!

  1. We protect ourselves from logical conflicts: cases where develop was passing before, but someone merged something in the meantime and now your change is broken on develop - and you don't find out until after you merge. We hit this almost every month.

One downside: before, clicking merge would happen instantly (although you had manually to wait for tests to be green). With this PR, you won't have to wait, but it will take 16 minutes to merge (plus, 16 more to put in the merge queue if you just pushed). But github will do the work itself and ensure tests are passing on the latest main.

@blaginin blaginin self-assigned this Apr 29, 2026
Comment on lines -36 to -40
paths-ignore:
- "docs/**"
- "**.md"
- ".github/ISSUE_TEMPLATE/**"
- ".github/pull_request_template.md"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit annoying that we'll run those on every pr even on those that changes docs. but:

  • we don't have many of those anyway
  • i really tried to fix this, but it makes the CI very complicated to install and support in the future

@github-actions github-actions Bot added the development-process Related to development process of DataFusion label Apr 29, 2026
@blaginin blaginin requested a review from alamb April 29, 2026 22:24
@blaginin
Copy link
Copy Markdown
Member Author

blaginin commented Apr 29, 2026

Now it will take 2x16 minutes to perform a merge (16 mins to put the PR in the queue + 16 mins to merge in the queue). I think that's OK since it's all automated. But we can actually make it MUCH faster - 2x6 mins instead.

tg_image_2949983809

You can see that the actual bottleneck is the Mac OS tests - that's because runs-on doesn't support them. My ideas:

  • kill the Mac OS tests. Although every three months or so there's 1-2 PRs that fail JUST on them, so probably not a good idea.
  • run macos tests just for a subset of tests, not everything?
  • introduce an "Optional Mac OS Check": this will be a no-on on push to a PR, but will run when the PR is in the merge queue. So instead of 2x16 mins it'll be just 1x16 mins (only in the merge queue). We can do the same with extended tests, which currently aren't required and only run on develop in full.
  • work with the infra team to get access to more performant github runners. Can take a while but i'm happy to do that!

@comphead comphead changed the title Rust checks are required + MQ chore: Rust checks are required + merge queue May 2, 2026
@comphead
Copy link
Copy Markdown
Contributor

comphead commented May 2, 2026

thanks @blaginin
another micro optimization prob is to check 0 tests

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

the execution takes nothing but planning might take something, 14 entries for linux tests and 20 for macos

@blaginin
Copy link
Copy Markdown
Member Author

blaginin commented May 6, 2026

thanks! @comphead

another micro optimization prob
yeah for sure, let's merge this and then i'll optimize that on top?

@blaginin blaginin requested a review from comphead May 6, 2026 14:21
Copy link
Copy Markdown
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @blaginin lets give it a try

@comphead comphead added this pull request to the merge queue May 6, 2026
Merged via the queue into apache:main with commit 5c097fd May 6, 2026
40 checks passed
@alamb
Copy link
Copy Markdown
Contributor

alamb commented May 6, 2026

woogoo!

@blaginin
Copy link
Copy Markdown
Member Author

blaginin commented May 6, 2026

image

noticed that, will clean up now

@blaginin
Copy link
Copy Markdown
Member Author

blaginin commented May 6, 2026

actually, it's actually expected - since the required checks became required AFTER we merged this PR, so the new ones should be working correctly!

diegoQuinas pushed a commit to diegoQuinas/datafusion that referenced this pull request May 6, 2026
apache#22048)

Follow up apache#21941

The macos-aarch64 job took ~22 min - 68% compile, 25% test. An audit of
macos-only PR failures over Mar-May 2026 (1000 failed PR runs, 30 with
macOS as the sole signal) found that nearly all the unique signal was
either:
- flaky sqllogictest metrics (~67%, e.g. push_down_filter /
explain_analyze .slt files with platform-dependent scan_efficiency_ratio
numbers),
  - real bugs in datafusion-benchmarks (which amd64 was excluding), or
- one genuinely macOS-specific FFI cdylib loading bug
(datafusion/ffi/src/tests/utils.rs path resolution).

So the only thing macOS uniquely catches is FFI dylib loading. Scope the
macOS job down to `cargo test -p datafusion-ffi --features
integration-tests` and let amd64 pick up the datafusion-benchmarks
coverage that was being dropped by the linux-test --exclude list.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable merge queue in github to avoid commit confliction.

3 participants